Skip to content

APIScan remediation: remove obsolete SharedTokenCacheUsername, fix masked CS0618 pragma, migrate obsolete ManagedIdentityCredential ctor - #4421

Merged
paulmedynski merged 2 commits into
mainfrom
dev/paul/wi-43668-remediation
Jul 16, 2026
Merged

paulmedynski merged 2 commits into
mainfrom
dev/paul/wi-43668-remediation

Conversation

@paulmedynski

Copy link
Copy Markdown
Contributor

Description

APIScan remediation for Microsoft.Data.SqlClient.Extensions.Azure
(ActiveDirectoryAuthenticationProvider.cs). Three related changes:

  1. Remove obsolete SharedTokenCacheUsername assignment (APIScan WI 42859 netstandard2.0 / WI 42860 net462 — same source line).

    • DefaultAzureCredentialOptions.SharedTokenCacheUsername is [Obsolete] + [EditorBrowsable(Never)] in the repo-pinned Azure.Identity (1.18.0) and undocumented on learn.microsoft.com.
    • SharedTokenCacheCredential is no longer in DefaultAzureCredential's default chain, so the assignment was a no-op. The client id is still propagated via ManagedIdentityClientId and WorkloadIdentityClientId for the in-chain credentials.
  2. Fix a masked CS0618 pragma. An unbalanced #pragma warning disable CS0618 (a second disable where a restore was intended) left obsolete-member warnings suppressed for the remainder of the file. Corrected to #pragma warning restore CS0618 so obsolete usages surface again (the project builds with TreatWarningsAsErrors=true).

  3. Migrate a now-unmasked obsolete API. Fixing the pragma exposed a genuine obsolete usage: ManagedIdentityCredential(string clientId, TokenCredentialOptions). Migrated to the supported ManagedIdentityCredential(ManagedIdentityCredentialOptions) constructor.

API changes / backwards compatibility

No public API changes. Behavior of the managed-identity path is preserved exactly:

  • Identity selection uses string.IsNullOrEmpty(clientId) → null-or-empty client id maps to ManagedIdentityId.SystemAssigned, otherwise ManagedIdentityId.FromUserAssignedClientId(clientId) — matching the obsolete constructor's internal logic.
  • AuthorityHost is carried over via ManagedIdentityCredentialOptions (which derives from TokenCredentialOptions).
  • The shared TokenCredentialOptions local is moved into the ClientSecretCredential branch, its only remaining consumer.

Issues

APIScan work items WI 42859 / WI 42860 (obsolete SharedTokenCacheUsername). Branch tracks WI 43668 remediation.

Testing

  • Project rebuilds clean with TreatWarningsAsErrors=true on net462 and netstandard2.0 (0 warnings / 0 errors). Before the pragma fix, the obsolete usages were silently compiled; after it, they correctly fail the build unless remediated — verified by temporarily reintroducing the removed line and observing error CS0618.
  • Azure.Test suite run: 21 passed, 9 skipped. The 2 failures are ActiveDirectoryInteractiveTests.TestConnection, which require a live interactive Entra ID sign-in against a real server and fail in a headless environment; they are unrelated to this change (which only touches credential construction, not interactive auth).

Notes

Draft while CI validates across all target frameworks.

paulmedynski and others added 2 commits July 2, 2026 14:59
…/42860)

Azure.Identity's DefaultAzureCredentialOptions.SharedTokenCacheUsername is
[Obsolete] + [EditorBrowsable(Never)] in the repo-pinned Azure.Identity and is
undocumented on learn.microsoft.com. SharedTokenCacheCredential is no longer in
DefaultAzureCredential's default chain, so the assignment is a no-op. Remove it to
resolve the APIScan CallReportingNotDocumentedMessage true positives (WI 42859
netstandard2.0, WI 42860 net462 -- same source line). ManagedIdentityClientId and
WorkloadIdentityClientId still carry the client id for the in-chain credentials.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…al ctor

An unbalanced '#pragma warning disable CS0618' (a second 'disable' where a
'restore' was intended) left obsolete-member warnings suppressed for the
remainder of ActiveDirectoryAuthenticationProvider.cs. This masked a genuine
obsolete usage: ManagedIdentityCredential(string clientId, TokenCredentialOptions).

- Correct the stray pragma to '#pragma warning restore CS0618' so obsolete
  warnings are surfaced again (the project builds with TreatWarningsAsErrors).
- Migrate the now-unmasked call to the supported
  ManagedIdentityCredential(ManagedIdentityCredentialOptions) constructor.
  Behavior is preserved exactly: identity selection uses string.IsNullOrEmpty
  (null or empty client id => ManagedIdentityId.SystemAssigned, otherwise
  ManagedIdentityId.FromUserAssignedClientId), matching the obsolete ctor, and
  AuthorityHost is carried over via the derived ManagedIdentityCredentialOptions
  (which is-a TokenCredentialOptions). The shared TokenCredentialOptions local is
  moved into the ClientSecretCredential branch, its only remaining consumer.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 2, 2026 18:39
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board Jul 2, 2026
@paulmedynski paulmedynski added this to the 7.1.0-preview3 milestone Jul 2, 2026
@paulmedynski paulmedynski added the Code Health 💊 Issues/PRs that are targeted to source code quality improvements. label Jul 2, 2026
@paulmedynski paulmedynski moved this from To triage to In progress in SqlClient Board Jul 2, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR remediates APIScan findings in Microsoft.Data.SqlClient.Extensions.Azure by removing an obsolete/no-op Azure.Identity option assignment, correcting an unbalanced #pragma that was unintentionally suppressing obsolete warnings across the file, and migrating a now-unmasked obsolete ManagedIdentityCredential constructor to the supported options-based constructor.

Changes:

  • Removed obsolete DefaultAzureCredentialOptions.SharedTokenCacheUsername assignment (no longer effective in the default credential chain).
  • Fixed a masked warning suppression by changing an unintended second #pragma warning disable CS0618 to #pragma warning restore CS0618.
  • Replaced obsolete ManagedIdentityCredential(string, TokenCredentialOptions) usage with ManagedIdentityCredential(ManagedIdentityCredentialOptions), preserving system-assigned vs user-assigned selection semantics.

@codecov

codecov Bot commented Jul 3, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.07%. Comparing base (c30bbbc) to head (71de86f).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4421      +/-   ##
==========================================
- Coverage   65.78%   64.07%   -1.71%     
==========================================
  Files         286      282       -4     
  Lines       43696    66658   +22962     
==========================================
+ Hits        28745    42713   +13968     
- Misses      14951    23945    +8994     
Flag Coverage Δ
CI-SqlClient ?
PR-SqlClient-Project 64.07% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@paulmedynski
paulmedynski marked this pull request as ready for review July 3, 2026 15:15
@paulmedynski
paulmedynski requested a review from a team as a code owner July 3, 2026 15:15
@paulmedynski
paulmedynski enabled auto-merge (squash) July 3, 2026 15:15
@paulmedynski paulmedynski moved this from In progress to In review in SqlClient Board Jul 3, 2026
@paulmedynski
paulmedynski disabled auto-merge July 3, 2026 16:28
@paulmedynski paulmedynski added Hotfix 7.0.3 PRs targeting main that should be backported to release/7.0 branch for next release. Hotfix 6.1.7 PRs targeting main that should be backported to release/6.1 branch for future hotfix labels Jul 3, 2026
@paulmedynski
paulmedynski enabled auto-merge (squash) July 13, 2026 10:56
@paulmedynski
paulmedynski merged commit e40ff48 into main Jul 16, 2026
367 of 368 checks passed
@paulmedynski
paulmedynski deleted the dev/paul/wi-43668-remediation branch July 16, 2026 11:19
@github-project-automation github-project-automation Bot moved this from In review to Done in SqlClient Board Jul 16, 2026
paulmedynski added a commit that referenced this pull request Jul 16, 2026
…e obsolete ManagedIdentityCredential ctor (#4421)

Cherry-pick of #4421 into release/6.1 (6.1.7). On this branch the provider
lives in the Microsoft.Data.SqlClient project rather than the
Microsoft.Data.SqlClient.Extensions.Azure project.

- Remove the obsolete DefaultAzureCredentialOptions.SharedTokenCacheUsername
  assignment (and its now-unneeded CS0618 pragma wrapper).
- Migrate the obsolete ManagedIdentityCredential(string, TokenCredentialOptions)
  constructor to ManagedIdentityCredential(ManagedIdentityCredentialOptions).

The masked-CS0618-pragma fix from #4421 does not apply here: the pragma is
already balanced (restore) on this branch.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
priyankatiwari08 added a commit that referenced this pull request Aug 26, 2026
Remove entries with no customer-visible effect, and reframe entries that
led with implementation detail rather than customer impact.

Removed:
- #3862 ConnectionCapabilities consolidation - internal refactor; the
  GetSchema("DataTypes") fix it enables is deferred to a later PR.
- #3700/#3741 SSRP scaffolding - adds no parsing code and no behavior change.
- #4517 CodeQL findings - PKCS#1 half is suppression comments for declared
  false positives; the SHA-1 removal is a no-op in practice.
- #4421 Extensions.Azure APIScan remediation - no public API change and
  managed-identity behavior preserved exactly; the removed assignment was
  already a no-op. Extensions.Azure now has no Changed section.

Promoted:
- #4504 counter fixes moved from a pool V2 sub-bullet into Fixed. These
  affect the default pool that customers use without opting in, and the
  previous text understated them as two fixes limited to Count.

Reframed:
- Cross-platform build collapsed to the one customer-visible outcome
  (trimming on Linux/macOS); package contents were always unchanged.
- #4528 now leads with the allocation regression rather than call-site count.
- AKV cache fixes now lead with symptoms (unbounded signature cache growth,
  duplicate CryptographyClient per key) rather than GetOrCreate/GetOrAdd.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8c691319-4c66-40f6-a88e-b453937052f1
priyankatiwari08 added a commit that referenced this pull request Aug 26, 2026
* Add release notes for 7.1.0-preview3

Adds release notes for Microsoft.Data.SqlClient 7.1.0-preview3 and its
four aligned companion packages, updates the per-version README index
tables, and adds the corresponding CHANGELOG entry.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c7425f3e-f6b8-439b-b74f-e25f8406fbf6

* Capture remaining closed 7.1.0-preview3 milestone items in release notes

Adds coverage for milestone items that closed after the initial draft:

- #4529 leaked-connection reclamation in ChannelDbConnectionPool
- #4535 SqlBulkCopy graph column alias mapping bypass
- #4521 / #4496 Entra ID tenant parsing for multi-segment STSURL authorities
- #4540 async key store provider APIs in the AzureKeyVaultProvider

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c485150e-f46e-4c55-8d06-e0491a10e7e8

* Add #4439 and #4445 to 7.1.0-preview3 release notes

Both are user-facing fixes merged into the preview3 milestone that were
not yet captured in the release notes or CHANGELOG:

- #4439: DateOnly values in sql_variant TVP columns were sent as datetime
  instead of date, overflowing for values outside the datetime range.
- #4445: ServerCertificate pin validation was skipped when the platform
  reported no TLS policy errors, and an unloadable certificate file fell
  back to host-name validation instead of failing closed.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8c691319-4c66-40f6-a88e-b453937052f1

* Add #4474 to 7.1.0-preview3 cross-platform build notes

PR #4474 (Remove OS-Specific Builds) removed OS-specific build targets
and output paths, and rewrote the MDS nuspec to source a single
OS-agnostic assembly for both the win and unix runtime folders.

Verified the nuspec change is src-path-only: all 52 file entries have
identical target= values before and after, so the produced package
layout is unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8c691319-4c66-40f6-a88e-b453937052f1

* Clarify #4536 allocation fix applies to the default async read path

Addresses review feedback on PR #4565: the PacketData node-reuse fix is
not gated behind UseCompatibilityAsyncBehaviour or any other AppContext
switch, and the perf validation was measured on the default path.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8c691319-4c66-40f6-a88e-b453937052f1

* Scope preview3 release notes to customer-facing changes

Remove entries with no customer-visible effect, and reframe entries that
led with implementation detail rather than customer impact.

Removed:
- #3862 ConnectionCapabilities consolidation - internal refactor; the
  GetSchema("DataTypes") fix it enables is deferred to a later PR.
- #3700/#3741 SSRP scaffolding - adds no parsing code and no behavior change.
- #4517 CodeQL findings - PKCS#1 half is suppression comments for declared
  false positives; the SHA-1 removal is a no-op in practice.
- #4421 Extensions.Azure APIScan remediation - no public API change and
  managed-identity behavior preserved exactly; the removed assignment was
  already a no-op. Extensions.Azure now has no Changed section.

Promoted:
- #4504 counter fixes moved from a pool V2 sub-bullet into Fixed. These
  affect the default pool that customers use without opting in, and the
  previous text understated them as two fixes limited to Count.

Reframed:
- Cross-platform build collapsed to the one customer-visible outcome
  (trimming on Linux/macOS); package contents were always unchanged.
- #4528 now leads with the allocation regression rather than call-site count.
- AKV cache fixes now lead with symptoms (unbounded signature cache growth,
  duplicate CryptographyClient per key) rather than GetOrCreate/GetOrAdd.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8c691319-4c66-40f6-a88e-b453937052f1

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c7425f3e-f6b8-439b-b74f-e25f8406fbf6
Copilot-Session: c485150e-f46e-4c55-8d06-e0491a10e7e8
Copilot-Session: 8c691319-4c66-40f6-a88e-b453937052f1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Code Health 💊 Issues/PRs that are targeted to source code quality improvements. Hotfix 6.1.7 PRs targeting main that should be backported to release/6.1 branch for future hotfix Hotfix 7.0.3 PRs targeting main that should be backported to release/7.0 branch for next release.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants